Complete Specifiers/Formulation separation (review of #672)#674
Conversation
Fixed 8 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
…rface - format specifier builder/map/home components so `format:check` (Static PR checks) passes again - add a `specifiers` search-command-surface config so the Specifiers mode composer renders smart examples, typeahead, and cross-mode chips instead of falling back to raw children - wire the existing Specifiers and Formulation mode action sets into the universal command surface so their quick actions render in each mode's composer - cover the Specifiers command-surface config with a targeted unit test Keeps Specifiers and Formulation as two fully separate modes, each connected to its own routes, search domain, and quick actions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C2VkQ6g1tYpMXCfpzKKkav
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughThis change introduces first-class Specifiers routes and UI, adds a psychiatric specifier catalogue and search tools, separates Specifiers from Formulation universal-search domains, updates navigation and sitemap ownership, and adds unit and browser coverage. ChangesSpecifier and Formulation separation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Heuristic only — a main-side or flake label is a starting point, not a verdict. |
The UI regression job was red on three stale/incorrect specifier assertions: - ui-smoke: the legacy test asserted `?mode=specifiers` redirects to `/formulation`. Specifiers is now its own mode, so it redirects to `/specifiers`; update the expectation to the standalone route and heading. - ui-specifiers "text-relevance" test used straight quotes in the results heading, but the page renders curly quotes (“/”); match them. - ui-specifiers builder test expected the pre-fix behaviour where a mixed deep link fell back to hard-coded MDD and disabled rapid cycling. The merged diagnosis-inference fix now infers a compatible bipolar base instead, so the deep link is scoped to the psychotic-features specifier that legitimately keeps the recurrent-MDD base and blocks the bipolar-only specifier. Verified locally against the running app: all 6 ui-specifiers tests and both mode-param redirect smoke tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C2VkQ6g1tYpMXCfpzKKkav
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ed63d4747
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
Summary
Testing
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/specifiers.ts (1)
632-646: 🎯 Functional Correctness | 🔵 TrivialSearch index omits
fit/notFit/checks/treatmentLens/wordingtext.
recordSearchTextonly indexes name/shortName/familyLabel/appliesTo/summary/clinicalSignal/decisionQuestion/patientLanguage/keywords. Clinically meaningful phrases that live only infit,notFit,checks,treatmentLens, orwording(e.g. distinguishing exclusion language) won't be found unless echoed inkeywords, reducing recall for a clinical search tool.♻️ Broaden the search haystack
function recordSearchText(record: SpecifierRecord) { return normalizeSearchText( [ record.name, record.shortName, record.familyLabel, ...record.appliesTo, record.summary, record.clinicalSignal, record.decisionQuestion, ...record.patientLanguage, ...record.keywords, + ...record.fit, + ...record.notFit, + ...record.checks, + record.treatmentLens, + record.wording, ].join(" "), ); }As per coding guidelines, P2-level observations for this file are consolidated here rather than posted as multiple inline notes ("P2 context belongs in one summary").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/specifiers.ts` around lines 632 - 646, Expand recordSearchText to include the clinically meaningful text from each record’s fit, notFit, checks, treatmentLens, and wording fields in the normalized search haystack, alongside the existing fields. Preserve the current normalization and joining behavior so phrases present only in those fields become searchable.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/specifiers/specifier-builder-page.tsx`:
- Around line 39-42: Update wordingSegment and the diagnosis composition flow so
severity is derived and emitted only once, preventing duplicate or conflicting
text when with-psychotic-features is combined with compatible severity or
remission records. Preserve the existing wording for non-conflicting records,
add targeted tests covering these combinations, then run the relevant domain
check and npm run check:production-readiness.
---
Nitpick comments:
In `@src/lib/specifiers.ts`:
- Around line 632-646: Expand recordSearchText to include the clinically
meaningful text from each record’s fit, notFit, checks, treatmentLens, and
wording fields in the normalized search haystack, alongside the existing fields.
Preserve the current normalization and joining behavior so phrases present only
in those fields become searchable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e690ac33-121c-4ffc-b292-eb08149a8dda
📒 Files selected for processing (34)
docs/branch-review-ledger.mddocs/site-map.mdplaywright.config.tsscripts/generate-site-map.tssrc/app/specifiers/[[...path]]/page.tsxsrc/app/specifiers/[slug]/page.tsxsrc/app/specifiers/builder/page.tsxsrc/app/specifiers/compare/page.tsxsrc/app/specifiers/layout.tsxsrc/app/specifiers/map/page.tsxsrc/app/specifiers/page.tsxsrc/components/clinical-dashboard/global-search-shell.tsxsrc/components/clinical-dashboard/master-search-header.tsxsrc/components/clinical-dashboard/mode-action-popup.tsxsrc/components/clinical-dashboard/universal-search-command-surface.tsxsrc/components/specifiers/specifier-builder-page.tsxsrc/components/specifiers/specifier-compare-page.tsxsrc/components/specifiers/specifier-map-page.tsxsrc/components/specifiers/specifier-record-page.tsxsrc/components/specifiers/specifier-ui.tsxsrc/components/specifiers/specifiers-home-page.tsxsrc/lib/app-mode-icons.tssrc/lib/search-command-surface.tssrc/lib/specifiers.tssrc/lib/universal-search-domains.tssrc/lib/universal-search-mode-context.tssrc/lib/universal-search.tstests/app-modes.test.tstests/search-command-surface.test.tstests/specifiers.test.tstests/ui-formulation.spec.tstests/ui-smoke.spec.tstests/ui-specifiers.spec.tstests/universal-search.test.ts
💤 Files with no reviewable changes (1)
- src/app/specifiers/[[...path]]/page.tsx
| function wordingSegment(record: SpecifierRecord) { | ||
| if (record.slug === "mild-severity") return "mild"; | ||
| if (record.slug === "with-psychotic-features") return "severe with psychotic features"; | ||
| return record.name.charAt(0).toLowerCase() + record.name.slice(1); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Compose severity only once in the generated diagnosis.
Selecting with-psychotic-features with a compatible severity/remission record can produce wording such as “severe with psychotic features, severe.” Normalization only excludes mild-severity; it does not prevent duplicate or conflicting severity language. Derive severity once and add targeted coverage for these combinations.
As per coding guidelines, “When behavior changes, add or update the smallest relevant targeted test” and run the relevant domain check plus npm run check:production-readiness.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/specifiers/specifier-builder-page.tsx` around lines 39 - 42,
Update wordingSegment and the diagnosis composition flow so severity is derived
and emitted only once, preventing duplicate or conflicting text when
with-psychotic-features is combined with compatible severity or remission
records. Preserve the existing wording for non-conflicting records, add targeted
tests covering these combinations, then run the relevant domain check and npm
run check:production-readiness.
Source: Coding guidelines
Summary
Reviews and completes PR #672's Specifiers/Formulation separation so both are two fully separate, working modes. This branch contains the separation work plus the fixes below:
/specifiers,/specifiers/[slug],/specifiers/builder,/specifiers/compare,/specifiers/map), each route connected to its own@/components/specifiers/*component and thespecifiersuniversal-search domain. The legacy/specifiers/[[...path]]redirect into Formulation is gone.formulationsearch domain — no cross-wiring between the two.specifier-builder-page.tsx,specifier-map-page.tsx, andspecifiers-home-page.tsxsoformat:check(Static PR checks) passes.specifiersentry tosearchCommandSurfaceByModeso the composer renders smart examples, typeahead, and cross-mode chips instead of falling back to raw children.Verification
npm run typechecknpm run lintnpm run format:checknpm run sitemap:checknpm run test— 2411 passed, 3 skippednpm run build— compiles; both/specifiers/*and/formulation/*route families emit as separate routesnpm run verify:ui— not run in this environment (Chromium UI gate); CI "UI regression" covers itClinical Governance Preflight
Change is limited to mode routing and search-composer wiring — no answer generation, retrieval/ranking logic, source rendering, ingestion, or privacy surface was modified.
Clinical KB Database(sjrfecxgysukkwxsowpy) (unchanged)Notes
codex/formulation-specifier-separation-current. The four CodeRabbit/Codex review findings (route metadata, builder deep-link diagnosis inference, mapinitialSlugresync, "clinical fit" → "text relevance" copy) were already applied in7a77dde; the outstanding one was the missing Specifiers command-surface config, now fixed here.🤖 Generated with Claude Code
https://claude.ai/code/session_01C2VkQ6g1tYpMXCfpzKKkav
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation